W-1 — _initialSelected no-op check regression — 🟢High [both reviewers]

lib/page/dashboard/views/dialogs/wifi_channel_dialog.dart — _onApply() (PR diff)

Scenario A (pre-disabled DFS state — Reviewer A):

// initState (PR diff):
_channels = filterDfsChannels(widget.radio.possibleChannels,
    band: widget.radio.band, dfsEnabled: widget.radio.isDfsEnabled);
// If isDfsEnabled=false AND channel=100 (DFS) AND autoChannelEnable=false:
//   → _channels does NOT contain 100
//   → storedChannelSelectable = false
//   → _selected = _autoValue = -1 ; _initialSelected = -1

// _onApply():
if (_selected == _initialSelected) {   // -1 == -1 → true
  Navigator.of(context).pop();         // returns null — NO write
  return;
}

If DFS was disabled in a previous session (before this PR's forceAutoChannelPaths logic ran, e.g. factory reset or pre-deployment firmware state) and the radio is still parked on a DFS channel, opening the dialog shows Auto (DFS channel filtered out). Tapping Apply without changing is a no-op → firmware stays with DFS=off, channel=100 (inconsistent). The old _onApply correctly detected this as a change (autoChannel=true != radio.autoChannelEnable=false) and would have written.

Scenario B (mid-dialog SSE model update — Reviewer B):

The old check compared against widget.radio (live). If an SSE event updates radio.channel mid-dialog (e.g. router auto-selects a non-DFS channel), the new _initialSelected-based comparison cannot detect this model change — it may either suppress a needed write or allow a spurious one depending on timing.

Fix (Scenario A): In initState, detect the DFS-forced-auto case:

final _dfsChannelWasForced = !widget.radio.isDfsEnabled &&
    !widget.radio.autoChannelEnable &&
    isDfsChannel(widget.radio.channel, band: widget.radio.band);

In _onApply, bypass the no-op check when _dfsChannelWasForced is true.

W-2 — DFS filtering inconsistency across data paths — 🟢High [single reviewer B]

Three filtering sites with different contracts:

1. usp_wifi_settings_service.dart:86-95: filters DFS before computeChannelsPerBandwidth → both possibleChannels and bandwidth maps are DFS-clean.
2. wifi_channel_dialog.dart:initState: filters DFS at dialog open time. WifiRadioUIModel.possibleChannels stores raw channels.
3. wifi_radio_ui_model.dart: possibleChannels is unfiltered on the dashboard path.

Any future consumer of WifiRadioUIModel.possibleChannels (tooltip, new feature) gets raw DFS channels regardless of DFS state, silently inconsistent with the settings path.

Fix: Document the contract explicitly on WifiRadioUIModel.possibleChannels. Consider filtering at model construction in usp_wifi_data_service.dart for consistency.

W-3 — setIeee80211hEnabled() USP result not parsed — 🟡Med [single reviewer A]

lib/page/wifi_settings/services/usp_wifi_advanced_service.dart

try {
  final params = <String, dynamic>{
    for (final path in radioPaths) '${path}IEEE80211hEnabled': enabled,
    for (final path in forceAutoChannelPaths)
      '${path}AutoChannelEnable': true,   // ← new in PR
  };
  await _usp.set(params);   // ← result discarded
} catch (e) {
  throw mapUspErrorToServiceError(e);
}

Contrast with usp_wifi_settings_service.dart:520-537 which uses UspResultParser.parseSetResult() and throws on UspPartialSuccess. If firmware accepts IEEE80211hEnabled=false but rejects AutoChannelEnable=true writes (partial failure), the UI shows DFS disabled while affected radios stay on DFS channels.

Fix: Parse result with UspResultParser.parseSetResult() and throw on UspPartialSuccess.

W-4 — TOCTOU: stale wifiDataProvider at save time — 🟡Med [single reviewer A]

lib/page/wifi_settings/providers/usp_wifi_advanced_provider.dart — performSave()

forceAutoChannelPaths is built from ref.read(wifiDataProvider) at save time. An SSE event (~500ms debounce) between the DFS toggle and the save could update wifiDataProvider, causing the path list to under-include (radio moved onto DFS channel mid-debounce) or over-include (spurious Auto-force, harmless).

Fix: Snapshot the radio state synchronously in setDfsEnabled() at toggle time before any async path.

W-5 — dfsChannels5GHz regulatory scope undocumented — 🟡Med [single reviewer B]

lib/core/utils/wifi_channel.dart

const Set<int> dfsChannels5GHz = {
  52, 56, 60, 64,
  100, 104, 108, 112, 116, 120, 124, 128, 132, 136, 140, 144,
};

This is the US/FCC UNII-2A/2C set. ETSI adds 120/124/128 to weather-radar restrictions (stricter CAC). MIC (Japan) has different DFS channel assignments entirely. Devices shipped to Japan would show wrong channel filtering.

Fix: Rename to dfsChannels5GhzUsFcc or add a /// doc comment stating // US/FCC (UNII-2A/2C) — extend per regulatory domain if multi-market required.

• Core DFS filter logic (filterDfsChannels, isDfsChannel, parsePossibleChannels): pure functions, well-implemented, comprehensive tests in test/core/utils/wifi_channel_test.dart (channel-0 sentinel, malformed range, inverted range, band gating — all covered).
• WifiRadioUIModel.isDfsEnabled: Equatable equality correctly derived via DiagnosticLoggable.namedProps (includes isDfsEnabled automatically — no explicit props override needed).
• Deduplication: Both private _parsePossibleChannels copies (in usp_wifi_data_service.dart and usp_wifi_settings_service.dart) correctly replaced by shared function per PR diff.
• forceAutoChannelPaths logic in performSave(): correctly handles the primary case (DFS being disabled NOW with radio on DFS channel); 2.4GHz/auto-channel/non-DFS-channel radios all correctly skipped.
• Test coverage: wifi_channel_test.dart (124 lines), DFS filter tests in usp_wifi_settings_service_test.dart, forceAutoChannelPaths notifier tests (4 cases), service test — thorough.
• Mutation lock discipline: withLock() preserved in performSave().
• No hardcoded secrets, no injection vectors, no privilege bypass paths.

W-1 (Rnd 2) — Two new async-throw tests missing await — High [both reviewers]

test/page/wifi_settings/services/usp_wifi_advanced_service_test.dart (approx. lines 183-215)

// Partial failure test
test('throws UspPartialFailureError on firmware partial rejection', () async {
  when(() => mockUsp.set(any())).thenAnswer((_) async => _setPartial());

  expect(           // <- missing await
    () => svc.setIeee80211hEnabled(
      radioPaths: ['Device.WiFi.Radio.1.', 'Device.WiFi.Radio.2.'],
      enabled: false,
      forceAutoChannelPaths: ['Device.WiFi.Radio.2.'],
    ),
    throwsA(isA<UspPartialFailureError>()),
  );
});

// Complete failure test
test('throws UspCompleteFailureError on complete failure', () async {
  when(() => mockUsp.set(any())).thenAnswer((_) async => _setFailure());

  expect(           // <- missing await
    () => svc.setIeee80211hEnabled(
      radioPaths: ['Device.WiFi.Radio.1.'],
      enabled: false,
    ),
    throwsA(isA<UspCompleteFailureError>()),
  );
});

setIeee80211hEnabled is async; expect(..., throwsA(...)) on an async closure itself returns Future<void>. Without await, Dart's test framework marks the test done before the Future resolves — the test passes regardless of whether the exception is ever thrown. These two tests are the sole guards for the W-3 fix (USP result parsing); any future regression in the parseSetResult/rethrow path would be silently missed.

Fix:

await expectLater(
  () => svc.setIeee80211hEnabled(...),
  throwsA(isA<UspPartialFailureError>()),
);
// and
await expectLater(
  () => svc.setIeee80211hEnabled(...),
  throwsA(isA<UspCompleteFailureError>()),
);

W-2 (Rnd 2) — allowPartial not set to true; UspPartialSuccess branch is dead code — High [single, Reviewer A]

lib/page/wifi_settings/services/usp_wifi_advanced_service.dart:65

final result = await _usp.set(params);   // allowPartial implicitly false
final parsed = UspResultParser.parseSetResult(result);
switch (parsed) {
  case UspSuccess(): break;
  case UspPartialSuccess(failures: final f):   // <- unreachable with allowPartial=false
    throw UspPartialFailureError(...);
  case UspFailure(errors: final e):
    throw UspCompleteFailureError(...);
}

Tracing through UspClientWasm._buildOptions (existing code):

JSAny? _buildOptions({bool allowPartial = false}) {
  if (!allowPartial) return null;   // options = null -> JS sees undefined -> atomic SET
  return {'allowPartial': allowPartial}.jsify();
}

With allowPartial=false (the default), the WASM client passes null options to firmware. Under TR-369, an atomic SET either fully succeeds (UspSuccess) or fully fails (UspFailure). There is no partial-success result. UspPartialSuccess is dead code.

Consequence: if firmware rejects AutoChannelEnable (e.g., radio is in a state that cannot switch), the entire atomic batch rolls back — including IEEE80211hEnabled. The DFS toggle silently appears to succeed on the UI while nothing was written. The code comment ("accepts IEEE80211hEnabled but rejects AutoChannelEnable") describes a scenario that is only possible with allowPartial: true.

Fix — two options:

• Option A (recommended): Pass allowPartial: true to match the intended semantics, and fix W-3 (Rnd 2) below to correctly populate successPaths.
• Option B: If atomic behaviour is intentional, remove the dead UspPartialSuccess branch and update the comment to reflect "atomic SET — either fully succeeds or fully fails".

W-3 (Rnd 2) — successPaths: const [] discards actual success paths — High [single, Reviewer B]

lib/page/wifi_settings/services/usp_wifi_advanced_service.dart:77

case UspPartialSuccess(failures: final f):
  throw UspPartialFailureError(
    summary: 'IEEE80211h update partial failure: ${f.first.errorMessage}',
    successPaths: const [],   // <- UspPartialSuccess.successes is discarded
    failures: f,
  );

UspPartialSuccess carries both successes (paths that were accepted) and failures (paths that were rejected). The pattern destructures only failures; successPaths is hard-coded empty.

15 other UspPartialFailureError call sites in the project (e.g., usp_wifi_settings_service.dart, local_network_service.dart, admin_service.dart) correctly pass:

successPaths: successes.map((s) => s.requestedPath).toList(),

Callers use successPaths to determine which paths were actually written (for partial UI refresh or targeted retry). An empty list causes the caller to treat the operation as if nothing succeeded.

Note: this issue is moot if W-2 (Rnd 2) is resolved via Option B (remove the dead branch). If choosing Option A (allowPartial: true), this must be fixed at the same time.

Fix:

case UspPartialSuccess(successes: final s, failures: final f):
  throw UspPartialFailureError(
    summary: 'IEEE80211h update partial failure: ${f.first.errorMessage}',
    successPaths: s.map((d) => d.requestedPath).toList(),
    failures: f,
  );

W-4 (Rnd 2) — Doc comment describes DFS pre-filtering that does not exist — Med [single, Reviewer B]

lib/page/_shared/models/wifi_radio_ui_model.dart (new comment lines 24-27)

/// (The WiFi Settings path pre-filters instead, in
/// `UspWifiSettingsService.buildWifiNetworks`, before building bandwidth
/// maps.)

Searching lib/ for filterDfsChannels returns zero hits — the function is not called anywhere in production code. UspWifiSettingsService.buildWifiNetworks calls _parsePossibleChannels, which returns raw channels without DFS filtering.

A future consumer reading this comment on the WiFi Settings path would conclude they can skip calling filterDfsChannels — leading to DFS channels being shown in a DFS-disabled context.

Fix: Correct the comment to reflect actual current behaviour:

/// NOTE: this list is RAW — it is NOT DFS-filtered on any code path.
/// Any consumer that must hide DFS channels (e.g., channel selection UI)
/// must call `filterDfsChannels` itself, keyed off [isDfsEnabled].

• W-2 fix (DFS filtering contract): possibleChannels doc comment correctly marks the field as RAW and directs consumers to call filterDfsChannels. Accurate for the dashboard path description. (Minor correction needed for the WiFi Settings path claim — see W-4 Rnd 2.)
• W-3 fix (USP result parsing) — production logic: UspResultParser.parseSetResult + if (e is ServiceError) rethrow pattern is correctly structured. The ServiceError guard prevents double-wrapping. Test reliability needs improvement (W-1 Rnd 2 above).
• W-5 fix (regulatory scope): dfsChannels5GHz now has a clear doc comment stating US/FCC scope and the need to extend for other regulatory domains.
• W-6 fix (ensureTrailingDot dedup): Clean extraction to lib/core/utils/tr181_path.dart. All 4 private implementations removed; all call sites migrated. Implementation is identical to the prior private copies — behaviorally correct. Placed in lib/core/utils/ (correct layer — pure string utility, no upstream imports).
• W-1/W-4 design decisions: Rationale accepted. View-only Apply intentionally not triggering a write is a documented design choice. TOCTOU window is narrow and over-include is harmless.
• Test additions (existing tests): 4 existing tests correctly updated from {} to _setSuccess() to match WASM v0.11.0 result shape. The _setPartial and _setFailure helper structures are correct (proper success field + error map shape).

• W-4 (Rnd 1) — Misleading doc comment (WiFi Settings path pre-filters DFS): Verified in HEAD — UspWifiSettingsService.buildWifiNetworks now calls filterDfsChannels with isDfsEnabled keyed off radio.ieee80211hEnabled. Doc comment on possibleChannels is accurate. ✅ Closed.
• W-6 (Rnd 1) — Duplicated ensureTrailingDot: Extracted to lib/core/utils/tr181_path.dart; all 4 private copies (usp_ethernet_data_service, usp_wifi_data_service, usp_wifi_settings_service, usp_wifi_advanced_provider) removed and all call sites migrated. ✅ Closed.
• W-5 (Rnd 1) — dfsChannels5GHz regulatory scope undocumented: Doc note added to wifi_channel.dart calling out US/FCC UNII-2A/2C scope and ETSI/MIC differences. ✅ Closed.

W-1 (Rnd 2, persists) — Two async-throw tests missing await — 🟢High [both reviewers]

test/page/wifi_settings/services/usp_wifi_advanced_service_test.dart:191,204

// Line 191 — partial rejection test
expect(
  () => svc.setIeee80211hEnabled(         // returns Future<void>
    radioPaths: ['Device.WiFi.Radio.1.', 'Device.WiFi.Radio.2.'],
    enabled: false,
    forceAutoChannelPaths: ['Device.WiFi.Radio.2.'],
  ),
  throwsA(isA<UspPartialFailureError>()),  // ← no await — Future completes after test exits
);

// Line 204 — complete failure test
expect(
  () => svc.setIeee80211hEnabled(
    radioPaths: ['Device.WiFi.Radio.1.'],
    enabled: false,
  ),
  throwsA(isA<UspCompleteFailureError>()), // ← same issue
);

svc.setIeee80211hEnabled is async; expect(fn, throwsA(...)) on an async closure returns a void (not Future<void>). The Dart test framework marks the test done before the returned Future resolves — both tests are permanently false-green. Any regression in the parseSetResult/rethrow path is silently missed. These two tests are the sole guards for the W-3 USP result-parsing fix.

Fix:

await expectLater(
  () => svc.setIeee80211hEnabled(
    radioPaths: ['Device.WiFi.Radio.1.', 'Device.WiFi.Radio.2.'],
    enabled: false,
    forceAutoChannelPaths: ['Device.WiFi.Radio.2.'],
  ),
  throwsA(isA<UspPartialFailureError>()),
);
// and
await expectLater(
  () => svc.setIeee80211hEnabled(
    radioPaths: ['Device.WiFi.Radio.1.'],
    enabled: false,
  ),
  throwsA(isA<UspCompleteFailureError>()),
);

W-2 (Rnd 2, persists) — allowPartial not passed; UspPartialSuccess branch unreachable in production — 🟢High [single, Reviewer B]

lib/page/wifi_settings/services/usp_wifi_advanced_service.dart:65

final result = await _usp.set(params);  // allowPartial defaults to false
// ...
switch (parsed) {
  case UspSuccess(): break;
  case UspPartialSuccess(failures: final f):   // ← unreachable under atomic SET
    throw UspPartialFailureError(...);
  case UspFailure(errors: final e):
    throw UspCompleteFailureError(...);
}

Under TR-369 with allowPartial=false (the default), a batch SET is atomic: either all parameters are written (UspSuccess) or the entire batch is rejected (UspFailure). The UspPartialSuccess outcome — where some paths succeed and others are rejected — is only possible with allowPartial: true. The mock _setPartial() fixture directly returns the partial-success shape, bypassing this firmware constraint, so the tests pass without validating the real production path.

The PR's stated goal (handle "firmware accepts IEEE80211hEnabled but rejects AutoChannelEnable") describes exactly the scenario that requires allowPartial: true.

Fix — choose one:

• Option A (intended semantics): Pass allowPartial: true and also fix W-3 (successPaths) at the same time.
• Option B (atomic semantics): If atomic behaviour is intentional, remove the UspPartialSuccess branch and update the doc comment to reflect "atomic SET — either fully succeeds or fully fails."

W-3 (Rnd 2, persists) — successPaths: const [] discards actual success paths — 🟢High [both reviewers]

lib/page/wifi_settings/services/usp_wifi_advanced_service.dart:77

case UspPartialSuccess(failures: final f):
  throw UspPartialFailureError(
    summary: 'IEEE80211h update partial failure: ${f.first.errorMessage}',
    successPaths: const [],   // ← UspPartialSuccess.successes is never destructured
    failures: f,
  );

UspPartialSuccess carries both successes: List<UspSuccessDetail> (paths that were accepted) and failures. The pattern destructures only failures; successPaths is hard-coded empty. 15 other UspPartialFailureError call sites in the project correctly pass successes.map((s) => s.requestedPath).toList(). Callers that use successPaths for partial UI refresh or targeted retry will see an empty list and behave as if nothing succeeded.

Note: this issue is moot if W-2 is resolved via Option B (remove the dead branch). If choosing Option A (allowPartial: true), both W-2 and W-3 must be fixed together.

Fix:

case UspPartialSuccess(successes: final s, failures: final f):
  throw UspPartialFailureError(
    summary: 'IEEE80211h update partial failure: ${f.map((e) => e.errorMessage).join('; ')}',
    successPaths: s.map((d) => d.requestedPath).toList(),
    failures: f,
  );

• ensureTrailingDot refactor: Clean extraction to lib/core/utils/tr181_path.dart. All 4 private copies removed; all call sites migrated. Implementation is behaviorally identical to prior private copies. Placed correctly in lib/core/utils/ (pure string utility, no upstream imports).
• dfsChannels5GHz regulatory scope: Now explicitly documented as US/FCC UNII-2A/2C with a note to extend for ETSI/MIC multi-market support.
• possibleChannels doc comment: Accurately documents the raw/filtered split — dashboard filters at display time via WifiChannelDialog, WiFi Settings pre-filters in buildWifiNetworks. Verified in HEAD.
• if (e is ServiceError) rethrow guard: Correctly prevents double-wrapping of UspPartialFailureError/UspCompleteFailureError by the outer mapUspErrorToServiceError. Matches the pattern in sibling services.
• Architecture compliance: No 3-layer dependency violations introduced. UspWifiAdvancedNotifier uses ref.read for L1 data and wraps mutations in uspMutationLockProvider.withLock(). All provider hard rules satisfied.
• Existing test updates: 4 prior tests correctly updated from {} to _setSuccess() to match WASM v0.11.0 result shape.

W-1 — Two async-throw tests missing await/expectLater — 🟢High [both reviewers]

test/page/wifi_settings/services/usp_wifi_advanced_service_test.dart:185–209

// First new test (~line 185):
test('throws UspPartialFailureError on firmware partial rejection', () async {
  when(() => mockUsp.set(any())).thenAnswer((_) async => _setPartial());

  expect(                          // ← no await — Future never checked
    () => svc.setIeee80211hEnabled(
      radioPaths: ['Device.WiFi.Radio.1.', 'Device.WiFi.Radio.2.'],
      enabled: false,
      forceAutoChannelPaths: ['Device.WiFi.Radio.2.'],
    ),
    throwsA(isA<UspPartialFailureError>()),
  );  // test exits here; Future resolves after test is already done
});

// Second new test (~line 201):
test('throws UspCompleteFailureError on complete failure', () async {
  when(() => mockUsp.set(any())).thenAnswer((_) async => _setFailure());

  expect(                          // ← no await — Future never checked
    () => svc.setIeee80211hEnabled(
      radioPaths: ['Device.WiFi.Radio.1.'],
      enabled: false,
    ),
    throwsA(isA<UspCompleteFailureError>()),
  );
});

svc.setIeee80211hEnabled is async; expect(fn, throwsA(...)) on an async closure returns a Future<void> that represents the async assertion check. Without await, the test function returns before that Future resolves — both tests are permanently false-green regardless of whether the service actually throws. Remove the throw logic entirely and these tests still pass.

Fix:

await expectLater(
  () => svc.setIeee80211hEnabled(
    radioPaths: ['Device.WiFi.Radio.1.', 'Device.WiFi.Radio.2.'],
    enabled: false,
    forceAutoChannelPaths: ['Device.WiFi.Radio.2.'],
  ),
  throwsA(isA<UspPartialFailureError>()),
);
// and
await expectLater(
  () => svc.setIeee80211hEnabled(
    radioPaths: ['Device.WiFi.Radio.1.'],
    enabled: false,
  ),
  throwsA(isA<UspCompleteFailureError>()),
);

W-2 — allowPartial not passed; UspPartialSuccess branch unreachable in production — 🟢High [both reviewers]

lib/page/wifi_settings/services/usp_wifi_advanced_service.dart:65

final result = await _usp.set(params);  // allowPartial defaults to false

UspClient.set({bool allowPartial = false}) — with allowPartial=false (TR-369 atomic SET), the firmware either fully succeeds (UspSuccess) or fully fails (UspFailure). UspPartialSuccess is only possible with allowPartial: true. The PR's stated goal — "firmware accepts IEEE80211hEnabled but rejects a forced AutoChannelEnable" — describes exactly the scenario that requires allowPartial: true to be observable. The new UspPartialSuccess arm at lines 73–79 is dead code in production, and the W-1 tests can never exercise the real production path even after fixing await.

Fix — choose one:

• Option A (intended semantics): await _usp.set(params, allowPartial: true); — and fix W-3 simultaneously.
• Option B (atomic semantics): Remove UspPartialSuccess arm and add a code comment explaining the atomic-only design intent.

W-3 — successPaths: const [] discards partial-success path information — 🟢High [both reviewers]

lib/page/wifi_settings/services/usp_wifi_advanced_service.dart:77

case UspPartialSuccess(failures: final f):
  throw UspPartialFailureError(
    summary: 'IEEE80211h update partial failure: ${f.first.errorMessage}',
    successPaths: const [],   // ← UspPartialSuccess.successes never destructured
    failures: f,
  );

UspPartialSuccess carries successes: List<UspSuccessDetail> (which paths were accepted by firmware). The pattern match leaves .successes unbound and hard-codes successPaths: const []. 15 other UspPartialFailureError call sites in the project correctly pass successes.map((s) => s.requestedPath).toList(). Any caller relying on successPaths for rollback, partial UI refresh, or targeted retry will see an empty list.

Note: moot if W-2 is resolved via Option B (remove dead branch). Must fix both W-2 and W-3 together for Option A.

Fix (Option A):

case UspPartialSuccess(successes: final s, failures: final f):
  throw UspPartialFailureError(
    summary: 'IEEE80211h update partial failure: ${f.map((e) => e.errorMessage).join('; ')}',
    successPaths: s.map((d) => d.requestedPath).toList(),
    failures: f,
  );

W-4 (new) — .first on potentially empty list → StateError masked by outer catch — 🟡Med [single, Reviewer A]

lib/page/wifi_settings/services/usp_wifi_advanced_service.dart:76 and :82

case UspPartialSuccess(failures: final f):
  throw UspPartialFailureError(
    summary: '...${f.first.errorMessage}',   // line 76: StateError if f is empty
    ...
  );
case UspFailure(errors: final e):
  throw UspCompleteFailureError(
    summary: '...${e.first.errorMessage}',   // line 82: StateError if e is empty
    ...
  );

If the WASM bridge returns a non-null but empty error map ({}), UspResultParser._parseGenericResult iterates error.entries and produces zero UspErrorDetail objects — yielding UspPartialSuccess(successes:[], failures:[]) or UspFailure([]). Calling .first on an empty list throws StateError: No element. The outer catch (e) intercepts this, but since StateError is not ServiceError, mapUspErrorToServiceError(e) wraps it as a generic error, losing all diagnostic context. Low-probability (non-standard firmware response), but silently masked when triggered.

Fix:

summary: 'IEEE80211h update partial failure: ${f.isNotEmpty ? f.first.errorMessage : 'unknown partial failure'}',
// and
summary: 'IEEE80211h update failed: ${e.isNotEmpty ? e.first.errorMessage : 'unknown failure'}',

• W-6 (Rnd 1) — Duplicated ensureTrailingDot: Extracted to lib/core/utils/tr181_path.dart; all 4 private copies removed; all call sites migrated. ✅ Closed.
• W-5 (Rnd 1) — dfsChannels5GHz regulatory scope undocumented: Doc note added to wifi_channel.dart (US/FCC UNII-2A/2C, with ETSI/MIC extension note). ✅ Closed.
• W-2 (Rnd 1) — DFS-filtering contract (dashboard vs settings path) undocumented: possibleChannels doc comment now explicitly documents the raw/filtered split and instructs new consumers to call filterDfsChannels themselves. ✅ Closed.
• W-3 (Rnd 1) — setIeee80211hEnabled() USP SET result not parsed: UspResultParser.parseSetResult now called on the batch result; partial/complete firmware rejections surface as typed UspPartialFailureError/UspCompleteFailureError. ✅ Partially addressed (remaining gaps tracked as W-2/W-3 above).

• ensureTrailingDot refactor: Clean extraction to lib/core/utils/ (correct layer — pure String → String, no upstream deps). All 4 private copies removed across usp_wifi_data_service, usp_wifi_settings_service, usp_wifi_advanced_provider, usp_ethernet_data_service. Behaviorally identical to prior private copies.
• dfsChannels5GHz doc comment: Regulatory scope (US/FCC UNII-2A/2C) documented; multi-market extension note added.
• possibleChannels doc comment: Dashboard raw vs WiFi Settings pre-filtered split documented; new consumers correctly directed to call filterDfsChannels themselves.
• if (e is ServiceError) rethrow guard: Correctly prevents double-wrapping of typed service errors by the outer mapUspErrorToServiceError. Matches sibling service pattern.
• Architecture compliance: No layer violations introduced. ensureTrailingDot correctly placed in lib/core/utils/. All imports follow allowed dependency directions.

W-1 carry — Missing await/expectLater in both new async-throw tests — 🟢High [both reviewers]

test/page/wifi_settings/services/usp_wifi_advanced_service_test.dart:185-211

test('throws UspPartialFailureError on firmware partial rejection', () async {
  when(() => mockUsp.set(any())).thenAnswer((_) async => _setPartial());
  expect(                          // <- no await — Future never checked
    () => svc.setIeee80211hEnabled(
      radioPaths: ['Device.WiFi.Radio.1.', 'Device.WiFi.Radio.2.'],
      enabled: false,
      forceAutoChannelPaths: ['Device.WiFi.Radio.2.'],
    ),
    throwsA(isA<UspPartialFailureError>()),
  );  // test exits before Future resolves — permanently false-green
});

test('throws UspCompleteFailureError on complete failure', () async {
  when(() => mockUsp.set(any())).thenAnswer((_) async => _setFailure());
  expect(                          // <- same issue
    () => svc.setIeee80211hEnabled(
      radioPaths: ['Device.WiFi.Radio.1.'],
      enabled: false,
    ),
    throwsA(isA<UspCompleteFailureError>()),
  );
});

svc.setIeee80211hEnabled is async. expect(fn, throwsA(...)) returns a Future<void> for async closures; without await the test framework exits before that future resolves. Both tests pass regardless of whether the service throws. Fix:

await expectLater(
  () => svc.setIeee80211hEnabled(...),
  throwsA(isA<UspPartialFailureError>()),
);

W-2 carry — allowPartial not passed; UspPartialSuccess unreachable in production — 🟢High [both reviewers]

lib/page/wifi_settings/services/usp_wifi_advanced_service.dart:65

final result = await _usp.set(params);   // allowPartial defaults to false

UspClient.set({bool allowPartial = false}) — with allowPartial=false, the TR-369 atomic SET means the WASM bridge passes undefined for options; firmware either fully succeeds (UspSuccess) or fully fails (UspFailure). UspPartialSuccess (partial accept) is structurally impossible without allowPartial: true. The PR's stated goal — "firmware accepts IEEE80211hEnabled but rejects a forced AutoChannelEnable" — is exactly the partial-accept scenario requiring allowPartial: true. The new UspPartialSuccess arm at line 73 is dead code in production, and W-1 means tests can't catch this even after fixing await.

Fix — choose one:

• Option A (intended semantics): await _usp.set(params, allowPartial: true); — and fix W-3 simultaneously.
• Option B (atomic semantics): Remove UspPartialSuccess arm and add a code comment.

W-3 carry — successPaths: const [] discards partial-success path information — 🟢High [both reviewers]

lib/page/wifi_settings/services/usp_wifi_advanced_service.dart:77

case UspPartialSuccess(failures: final f):
  throw UspPartialFailureError(
    summary: 'IEEE80211h update partial failure: ${f.first.errorMessage}',
    successPaths: const [],   // <- UspPartialSuccess.successes never destructured
    failures: f,
  );

UspPartialSuccess carries successes: List<UspSuccessDetail>. Hard-coding successPaths: const [] means any caller relying on successPaths for rollback, partial UI refresh, or retry sees an empty list. 15 other UspPartialFailureError call sites in the project correctly pass successes.map((s) => s.requestedPath).toList(). Moot if W-2 is resolved via Option B.

Fix (Option A):

case UspPartialSuccess(successes: final s, failures: final f):
  throw UspPartialFailureError(
    summary: 'IEEE80211h update partial failure: ${f.isNotEmpty ? f.first.errorMessage : 'unknown'}',
    successPaths: s.map((d) => d.requestedPath).toList(),
    failures: f,
  );

W-4 carry — .first on potentially-empty list → StateError masked by outer catch — 🟢High [single]

lib/page/wifi_settings/services/usp_wifi_advanced_service.dart:76, 82

case UspPartialSuccess(failures: final f):
  throw UspPartialFailureError(
    summary: '...${f.first.errorMessage}',   // line 76: StateError if f is empty
    ...
  );
case UspFailure(errors: final e):
  throw UspCompleteFailureError(
    summary: '...${e.first.errorMessage}',   // line 82: StateError if e is empty
    ...
  );

When the WASM bridge returns {success: true/false, result: {data: ..., error: {}}} (error key present but empty map), _parseGenericResult's for (final entry in error.entries) loop body never executes — yielding UspPartialSuccess(successes, []) or UspFailure([]). Calling .first on an empty list throws StateError: No element. The outer catch (e) intercepts it, but since StateError is not ServiceError, mapUspErrorToServiceError wraps it as a generic error, losing all diagnostic context.

Fix:

summary: f.isNotEmpty
    ? 'IEEE80211h update partial failure: ${f.first.errorMessage}'
    : 'IEEE80211h update partial failure (no error detail)',
// and for UspFailure:
summary: e.isNotEmpty
    ? 'IEEE80211h update failed: ${e.first.errorMessage}'
    : 'IEEE80211h update failed (no error detail)',

W-NEW-1 — _setFailure stub key matches parser fallback; semantic ambiguity — 🟡Med [single]

test/page/wifi_settings/services/usp_wifi_advanced_service_test.dart:30-43

Map<String, dynamic> _setFailure({
  String path = 'bulk_operation',   // <- same string as parser's fallback requestedPath
  ...
}) => {'success': false, 'result': {'data': {}, 'error': {path: {'errorCode': ...}}}};

UspResultParser._parseGenericResult uses 'bulk_operation' as the fallback requestedPath when error == null. The _setFailure stub uses the exact same string as the error-map key, so the test is indistinguishable from the null-error fallback path. Fix: use a realistic path like 'Device.WiFi.Radio.1.IEEE80211hEnabled' as the error key.

W-NEW-2 — UspPartialFailureError rethrow path has zero real test coverage — 🟡Med [single]

lib/page/wifi_settings/services/usp_wifi_advanced_service.dart:86-88

} catch (e) {
  if (e is ServiceError) rethrow;   // UspPartialFailureError IS ServiceError -- correct
  throw mapUspErrorToServiceError(e);
}

Logic is correct (UspPartialFailureError extends ServiceError), but the rethrow path cannot be exercised by any test until W-1 is fixed. No production code change needed; fix W-1 first.

W-NEW-3 — errorCode typed as int in stubs; WASM JSON may return String — 🟡Med [single]

test/page/wifi_settings/services/usp_wifi_advanced_service_test.dart:24-28

Map<String, dynamic> _setPartial({int errorCode = 7008, ...}) => {
  'error': {path: {'errorCode': errorCode, ...}}  // Dart int literal
};

UspResultParser casts errorInfo['errorCode'] as int?. JavaScript has no native int; if the WASM bridge returns this as a String, the cast throws CastError. The int-literal stubs can't surface this mismatch. Fix suggestion: defensive parsing: int.tryParse(map['errorCode'].toString()) ?? -1.

Reviewer B flagged Critical-1: "migration of ensureTrailingDot incomplete — private copies still exist in service files." Independent verification against the actual PR head (git show pr-1124-head) confirms all three service files (usp_ethernet_data_service.dart, usp_wifi_data_service.dart, usp_wifi_settings_service.dart) correctly use ensureTrailingDot with their private copies removed. Reviewer B was checking local staging state rather than the PR head. Confirmed false positive — not a real issue.

• ensureTrailingDot extraction complete: All 4 private copies removed; all call sites migrated to lib/core/utils/tr181_path.dart. Layer placement correct (pure String -> String in core/utils/).
• possibleChannels doc comment: Raw vs pre-filtered split documented; new consumers correctly directed to call filterDfsChannels themselves.
• dfsChannels5GHz regulatory scope note: US/FCC UNII-2A/2C set documented with multi-market extension note.
• UspResultParser.parseSetResult call added: Firmware partial rejection now surfaces as typed errors instead of being silently swallowed — intent is correct.
• if (e is ServiceError) rethrow guard: Correctly prevents double-wrapping. Matches sibling service pattern.
• Architecture compliance: No layer violations. All imports follow allowed dependency directions.
• Test stubs structurally sound: _setSuccess()/_setPartial()/_setFailure() shapes are correct WASM response structures; existing tests correctly updated from {} to _setSuccess().

W-1 carry — Missing await/expectLater in both async-throw tests — 🟢High [both reviewers]

test/page/wifi_settings/services/usp_wifi_advanced_service_test.dart:191, 204

// partial failure test (lines 185–198)
expect(
  () => svc.setIeee80211hEnabled(
    radioPaths: ['Device.WiFi.Radio.1.', 'Device.WiFi.Radio.2.'],
    enabled: false,
    forceAutoChannelPaths: ['Device.WiFi.Radio.2.'],
  ),
  throwsA(isA<UspPartialFailureError>()),
);  // ← no await — Future never checked; permanently false-green

// complete failure test (lines 201–210)
expect(
  () => svc.setIeee80211hEnabled(
    radioPaths: ['Device.WiFi.Radio.1.'],
    enabled: false,
  ),
  throwsA(isA<UspCompleteFailureError>()),
);  // ← same issue

svc.setIeee80211hEnabled is async. expect(() => asyncFn(), throwsA(...)) returns a Future<void> for async closures; without await the framework exits before the Future resolves. Both tests pass regardless of whether the service throws — even if the entire switch (parsed) block were deleted.

Fix: await expectLater(() => svc.setIeee80211hEnabled(...), throwsA(isA<UspPartialFailureError>()));

W-2 carry — allowPartial not passed; UspPartialSuccess branch structurally unreachable — 🟢High [both reviewers]

lib/page/wifi_settings/services/usp_wifi_advanced_service.dart:65

final result = await _usp.set(params);   // allowPartial defaults to false

UspClientWasm._buildOptions() returns null when allowPartial: false (JS sees undefined). With atomic SET semantics, firmware either fully succeeds (UspSuccess) or fully fails (UspFailure). UspPartialSuccess is structurally impossible without allowPartial: true. The partial-success arm at lines 73–79 is dead code in production. W-1 means tests cannot catch this even after fixing await.

Fix — choose one:

• Option A (intended semantics): await _usp.set(params, allowPartial: true); and fix W-3 simultaneously.
• Option B (atomic semantics): Remove UspPartialSuccess arm and add a code comment.

W-3 carry — successPaths: const [] discards partial-success path information — 🟢High [both reviewers]

lib/page/wifi_settings/services/usp_wifi_advanced_service.dart:77

case UspPartialSuccess(failures: final f):
  throw UspPartialFailureError(
    summary: 'IEEE80211h update partial failure: ${f.first.errorMessage}',
    successPaths: const [],   // ← UspPartialSuccess.successes never destructured
    failures: f,
  );

UspPartialSuccess carries successes: List<UspSuccessDetail>. Hard-coding const [] means any caller relying on successPaths for rollback or partial UI refresh sees an empty list. 15 other UspPartialFailureError call sites correctly pass successes.map((s) => s.requestedPath).toList(). Moot if W-2 is resolved via Option B.

Fix (if Option A):

case UspPartialSuccess(successes: final s, failures: final f):
  throw UspPartialFailureError(
    summary: 'IEEE80211h update partial failure: ${f.firstOrNull?.errorMessage ?? 'unknown'}',
    successPaths: s.map((d) => d.requestedPath).toList(),
    failures: f,
  );

W-4 carry — .first on potentially-empty list → StateError masked by outer catch — 🟢High [single]

lib/page/wifi_settings/services/usp_wifi_advanced_service.dart:76, 82

case UspPartialSuccess(failures: final f):
  throw UspPartialFailureError(
    summary: '...${f.first.errorMessage}',   // StateError if f is empty
    ...);
case UspFailure(errors: final e):
  throw UspCompleteFailureError(
    summary: '...${e.first.errorMessage}',   // StateError if e is empty
    ...);

If WASM returns {success: true/false, result: {data: {}, error: {}}} — error key present but empty map — the for loop in _parseGenericResult never runs, yielding UspPartialSuccess(successes, []) or UspFailure([]). .first on an empty list throws StateError. Since StateError is not ServiceError, the outer catch wraps it via mapUspErrorToServiceError, losing all diagnostic context.

Fix: f.isNotEmpty ? f.first.errorMessage : 'unknown' or f.firstOrNull?.errorMessage ?? 'unknown'.

W-NEW-1 — _setFailure stub key 'bulk_operation' matches parser internal fallback — 🟡Med [single]

test/page/wifi_settings/services/usp_wifi_advanced_service_test.dart:30–43

Map<String, dynamic> _setFailure({
  String path = 'bulk_operation',   // matches UspResultParser's synthetic fallback key
  ...
})

UspResultParser._parseGenericResult uses 'bulk_operation' as the synthetic requestedPath when the error map is null. The stub passes a real error map keyed by the same string — technically a different code path (explicit error present vs null-error fallback) but indistinguishable by reading the test. Fix: use a realistic path like 'Device.WiFi.Radio.1.IEEE80211hEnabled' as the error key.

W-NEW-2 — UspPartialFailureError rethrow path has zero real test coverage — 🟡Med [single]

lib/page/wifi_settings/services/usp_wifi_advanced_service.dart:86–88

The if (e is ServiceError) rethrow; guard is logically correct (UspPartialFailureError extends ServiceError), but the rethrow path cannot be exercised by any test until W-1 is fixed. No production code change needed; fix W-1 first.

W-NEW-3 — errorCode typed as int in stubs; WASM JSON may return String — 🟡Med [single]

test/page/wifi_settings/services/usp_wifi_advanced_service_test.dart:15–28 + lib/core/usp/models/usp_operation_result.dart:421

int errorCode = 7008,   // Dart int literal in stub

// parser:
errorCode: errorInfo['errorCode'] as int? ?? -1,  // ← throws TypeError if WASM gives String

If WASM returns "errorCode": "7008" (JSON string), as int? throws TypeError (not caught by ?? -1). The int-literal stubs can't surface this mismatch. Fix: (errorInfo['errorCode'] as num?)?.toInt() ?? int.tryParse(errorInfo['errorCode'].toString()) ?? -1.

NEW-A — Provider computes DFS channel remediation logic — 🟡Med [single, Reviewer B]

lib/page/wifi_settings/providers/usp_wifi_advanced_provider.dart:117–125

// UspWifiAdvancedNotifier.performSave (Provider layer):
final radios = ref.read(wifiDataProvider).valueOrNull?.radioModels ?? [];
final radioByPath = {
  for (final r in radios) ensureTrailingDot(r.instancePath): r,
};
for (final path in radioPaths) {
  if (current.ieee80211hByRadio[path] == true) continue;
  final radio = radioByPath[ensureTrailingDot(path)];
  if (radio == null || radio.autoChannelEnable) continue;
  if (isDfsChannel(radio.channel, band: radio.band)) {
    forceAutoChannelPaths.add(path);   // ← business rule in Provider
  }
}

The decision "which radios need forceAutoChannelPaths" is a business rule (DFS channel remediation) embedded in performSave. This couples the Provider to WiFi radio state details and requires a full Provider environment (with wifiDataProvider mock) just to test the DFS logic. Fix suggestion: extract to a pure function computeForceAutoChannelPaths(...) in the Service layer or core/utils/.

• ensureTrailingDot unification complete: All 4 private copies (usp_ethernet_data_service, usp_wifi_data_service, usp_wifi_settings_service, usp_wifi_advanced_provider) removed; all call sites migrated to lib/core/utils/tr181_path.dart. Layer placement correct (core/utils/ — pure String → String helper).
• possibleChannels doc note: Clear documentation of the RAW/pre-filtered split and consumer responsibility for filterDfsChannels. Prevents future consumers from silently exposing DFS channels.
• dfsChannels5GHz regulatory scope note: US/FCC UNII-2A/2C scope documented with multi-market extension guidance.
• UspResultParser.parseSetResult integration: Architecture direction correct — firmware partial/complete rejection now surfaces as typed errors instead of being silently swallowed.
• if (e is ServiceError) rethrow guard: Correctly prevents double-wrapping. Matches sibling service pattern.

W-1 carry — Missing await/expectLater in both new async-throw tests — 🟢High [both reviewers]

test/page/wifi_settings/services/usp_wifi_advanced_service_test.dart:191, 204

// Line 191 — partial rejection test
expect(
  () => svc.setIeee80211hEnabled(
    radioPaths: ['Device.WiFi.Radio.1.', 'Device.WiFi.Radio.2.'],
    enabled: false,
    forceAutoChannelPaths: ['Device.WiFi.Radio.2.'],
  ),
  throwsA(isA<UspPartialFailureError>()),
);  // ← no await — Future never checked; permanently false-green

// Line 204 — complete failure test
expect(
  () => svc.setIeee80211hEnabled(
    radioPaths: ['Device.WiFi.Radio.1.'],
    enabled: false,
  ),
  throwsA(isA<UspCompleteFailureError>()),
);  // ← same issue

svc.setIeee80211hEnabled is async. expect(() => asyncFn(), throwsA(...)) without await / expectLater discards the returned Future synchronously — both tests always pass regardless of whether the service throws. Note: Austin self-flagged these inline in his own review comments (lines 191, 204), confirming awareness. Fix: await expectLater(() => svc.setIeee80211hEnabled(...), throwsA(isA<UspPartialFailureError>()));

W-3 carry — successPaths: const [] discards partial-success path info — 🟢High [both reviewers]

lib/page/wifi_settings/services/usp_wifi_advanced_service.dart:77

case UspPartialSuccess(failures: final f):
  throw UspPartialFailureError(
    summary: 'IEEE80211h update partial failure: ${f.first.errorMessage}',
    successPaths: const [],   // ← UspPartialSuccess.successes never destructured
    failures: f,
  );

UspPartialSuccess carries successes: List<UspSuccessDetail>. Every other UspPartialFailureError construction in the codebase (12+ call sites verified) passes successPaths: s.map((d) => d.requestedPath).toList(). Hard-coding const [] silently discards which radios succeeded — information callers may need for diagnostics or partial rollback. Note: partially moot until W-2 is resolved (see contradictory finding).

Fix:

case UspPartialSuccess(successes: final s, failures: final f):
  throw UspPartialFailureError(
    summary: 'IEEE80211h update partial failure: ${f.firstOrNull?.errorMessage ?? 'unknown'}',
    successPaths: s.map((d) => d.requestedPath).toList(),
    failures: f,
  );

W-4 carry — .first on potentially-empty list → StateError masked by outer catch — 🟢High [both reviewers]

lib/page/wifi_settings/services/usp_wifi_advanced_service.dart:76, 82

case UspPartialSuccess(failures: final f):
  throw UspPartialFailureError(
    summary: '...${f.first.errorMessage}',   // StateError if f is empty
    ...);
case UspFailure(errors: final e):
  throw UspCompleteFailureError(
    summary: '...${e.first.errorMessage}',   // StateError if e is empty
    ...);

While UspResultParser._parseGenericResult populates at least one failure in practice, this is an internal parser detail, not a documented invariant on the typed fields. An empty-error-map response (error: {}) from firmware could yield UspFailure([]), causing .first to throw StateError. The outer catch (e) then re-wraps it as an opaque UnexpectedError, losing all diagnostic context.

Fix: f.firstOrNull?.errorMessage ?? 'unknown' — or use the pre-existing errorSummary getter on UspPartialSuccess/UspFailure which is safe on empty lists.

W-NEW-1 carry — _setFailure stub key 'bulk_operation' matches parser internal fallback — 🟡Med [both reviewers]

test/page/wifi_settings/services/usp_wifi_advanced_service_test.dart:31

Map<String, dynamic> _setFailure({
  String path = 'bulk_operation',   // matches UspResultParser's synthetic sentinel
  ...
})

UspResultParser._parseGenericResult synthesises requestedPath: 'bulk_operation' with errorMessage: 'Operation failed' when success=false AND error==null. The stub sends an explicit error map keyed to the same string — technically different parser paths but producing identical UspErrorDetail objects. A regression where WASM omits the error map entirely would produce the same assertion result, masking the parser's fallback path.

Fix: Use a realistic TR-181 path as default: String path = 'Device.WiFi.Radio.1.IEEE80211hEnabled'.

W-2 carry — allowPartial not passed [CONTRADICTORY — Austin please verify] — 🟢High [single, Reviewer B; Reviewer A disputes]

lib/page/wifi_settings/services/usp_wifi_advanced_service.dart:65

final result = await _usp.set(params);   // allowPartial defaults to false

Reviewer B (still-present): Reviewed usp_client_wasm.dart:194-196 — WASM only returns a mixed {success:true, data:{...}, error:{...}} shape when allowPartial: true is explicitly passed. With the default allowPartial: false, any parameter rejection causes WASM to return {success:false,...} → UspFailure. The UspPartialSuccess arm at lines 73–79 is structurally unreachable in production. Reference: usp_local_network_service.dart:75 demonstrates the correct pattern (allowPartial: true).

Reviewer A (resolved): The parseSetResult plumbing correctly detects partial-success shapes in the returned map — this is a valid alternative contract, and the branch is now reachable.

Two reviewers reached opposite conclusions. Please verify which WASM shape is actually emitted with allowPartial: false in the partial-rejection scenario.

NEW-A carry — Provider performSave() contains DFS channel remediation business logic — 🟢High [single, Reviewer B]

lib/page/wifi_settings/providers/usp_wifi_advanced_provider.dart:115–129

// In UspWifiAdvancedNotifier.performSave (Provider layer):
for (final path in radioPaths) {
  if (current.ieee80211hByRadio[path] == true) continue;
  final radio = radioByPath[ensureTrailingDot(path)];
  if (radio == null || radio.autoChannelEnable) continue;
  if (isDfsChannel(radio.channel, band: radio.band)) {   // ← domain logic in Provider
    forceAutoChannelPaths.add(path);
  }
}

PrivacyGUI constitution Article VI: business logic must live in page/[feature]/services/. The decision of which radios need forceAutoChannelPaths (the DFS channel remediation rule) is a business rule that couples the Provider to isDfsChannel() domain semantics. UspWifiAdvancedService.setIeee80211hEnabled already accepts forceAutoChannelPaths as a parameter, indicating the computation should be provided by the caller — but the computation itself should be in the Service, not the Provider.

Fix: Extract to UspWifiAdvancedService.computeDfsRemediationPaths(radioModels, ieee80211hByRadio), keeping the Provider ignorant of DFS semantics.

W-NEW-3 carry — errorCode stub uses int; WASM JSON may return String → parser TypeError — 🟡Med [single, Reviewer A]

test/page/wifi_settings/services/usp_wifi_advanced_service_test.dart:17, 32

Map<String, dynamic> _setPartial({int errorCode = 7008, ...})  // Dart int literal
// Parser in usp_operation_result.dart:
errorCode: errorInfo['errorCode'] as int? ?? -1,  // hard cast — TypeError if String

If WASM delivers "errorCode": "7008" (JSON string), as int? throws TypeError (not caught by ?? -1). The int-literal stubs cannot surface this mismatch.

Fix: (errorInfo['errorCode'] as num?)?.toInt() ?? int.tryParse(errorInfo['errorCode']?.toString() ?? '') ?? -1; or add a test variant with a string errorCode.

• ensureTrailingDot unification complete: All 4 private copies (usp_ethernet_data_service, usp_wifi_data_service, usp_wifi_settings_service, usp_wifi_advanced_provider) removed and migrated to lib/core/utils/tr181_path.dart. Layer placement correct (core/utils/ — pure String → String helper).
• UspResultParser.parseSetResult integration: Firmware partial/complete rejection for setIeee80211hEnabled now surfaces as typed errors instead of being silently swallowed.
• if (e is ServiceError) rethrow guard: Correctly prevents double-wrapping. Matches sibling service pattern.
• possibleChannels doc note: Clear documentation of the RAW/pre-filtered split and consumer responsibility for filterDfsChannels. Prevents future consumers from silently exposing DFS channels.
• dfsChannels5GHz regulatory scope note: US/FCC UNII-2A/2C scope documented with multi-market extension guidance.
• Test stubs upgraded: Existing mock.set stubs now return well-formed success-shaped _setSuccess() maps; new failure-path tests added (even if the await is missing).

W-3 carry — successPaths: const [] discards partial-success path info — 🟢High [both reviewers]

lib/page/wifi_settings/services/usp_wifi_advanced_service.dart:77

case UspPartialSuccess(failures: final f):
  throw UspPartialFailureError(
    summary: 'IEEE80211h update partial failure: ${f.first.errorMessage}',
    successPaths: const [],   // ← UspPartialSuccess.successes never destructured
    failures: f,
  );

UspPartialSuccess carries successes: List<UspSuccessDetail>. Every other UspPartialFailureError construction in the codebase passes successPaths: s.map((d) => d.requestedPath).toList(). Hard-coding const [] silently discards which radios succeeded — information callers may need for diagnostics or partial rollback.

Fix:

case UspPartialSuccess(successes: final s, failures: final f):
  throw UspPartialFailureError(
    summary: 'IEEE80211h update partial failure: ${f.firstOrNull?.errorMessage ?? 'unknown'}',
    successPaths: s.map((d) => d.requestedPath).toList(),
    failures: f,
  );

W-4 carry — .first on potentially-empty list → StateError masked by outer catch — 🟢High [both reviewers]

lib/page/wifi_settings/services/usp_wifi_advanced_service.dart:76, 82

case UspPartialSuccess(failures: final f):
  throw UspPartialFailureError(
    summary: '...${f.first.errorMessage}',   // StateError if f is empty
    ...);
case UspFailure(errors: final e):
  throw UspCompleteFailureError(
    summary: '...${e.first.errorMessage}',   // StateError if e is empty
    ...);

While UspResultParser._parseGenericResult populates at least one entry in practice, this is an internal parser invariant not reflected in the type signature. An unexpected firmware response (e.g., error: {} empty map) could yield an empty list; .first throws StateError. The outer catch (e) block's if (e is ServiceError) rethrow guard does not catch StateError, so mapUspErrorToServiceError(e) wraps it as an opaque UnexpectedError, losing all diagnostic context.

Fix: f.firstOrNull?.errorMessage ?? 'unknown' or use the existing errorSummary getter.

W-1 carry — Missing await/expectLater on async-throw tests — 🟢High [CONTRADICTORY — please verify]

test/page/wifi_settings/services/usp_wifi_advanced_service_test.dart:191, 204

expect(
  () => svc.setIeee80211hEnabled(...),
  throwsA(isA<UspPartialFailureError>()),
);  // ← no await

Reviewer A (retracted): Built a test reproducer and ran the full suite (+16 all pass). flutter_test detects async closures returning the wrong result type — the unawaited Future error propagates to the test zone and the test fails correctly. The prior "false-green" characterisation was incorrect. Both new tests are true-green.

Reviewer B (still-present): expect(fn, throwsA(...)) without await drops the returned Future<void> synchronously — the test exits before the assertion resolves. Idiomatic Dart test pattern is await expectLater(...).

Two reviewers reached opposite conclusions. Austin self-flagged these at lines 191 and 204 — please run the suite with a mock that does NOT throw and verify these tests actually fail.

NEW-W2 — forceAutoChannelPaths computation has no test coverage — 🟢High [single, Reviewer A]

lib/page/wifi_settings/providers/usp_wifi_advanced_provider.dart:116–131

final radioByPath = {
  for (final r in radios) ensureTrailingDot(r.instancePath): r,
};
for (final path in radioPaths) {
  if (current.ieee80211hByRadio[path] == true) continue;
  final radio = radioByPath[ensureTrailingDot(path)];
  if (radio == null || radio.autoChannelEnable) continue;
  if (isDfsChannel(radio.channel, band: radio.band)) {
    forceAutoChannelPaths.add(path);
  }
}

None of the three filtering conditions (DFS-already-enabled skip / radio == null guard / isDfsChannel gate) have a provider-level test verifying the correct paths reach setIeee80211hEnabled. A path-key mismatch in the double ensureTrailingDot application would cause radio to be always null, silently skipping all channel remediation.

Fix: Add a notifier test: populate wifiDataProvider with a radio on channel 52 (DFS), band 5GHz, autoChannelEnable: false; call save() with enabled=false; verify setIeee80211hEnabled is called with that radio path in forceAutoChannelPaths.

NEW-B4 — _parsePossibleChannels is a verbatim duplicate of wifi_channel.dart::parsePossibleChannels — 🟢High [single, Reviewer B]

lib/page/wifi_settings/services/usp_wifi_data_service.dart:401–432

Both functions have identical logic, identical doc-comment text, and identical edge-case handling. wifi_channel.dart is the shared utility this PR actively modifies — yet usp_wifi_data_service.dart was not migrated to call it. This is the same class of duplicate as _ensureTrailingDot (addressed for other helpers in this PR), leaving one dedup missed within the PR's stated scope.

Fix: Import wifi_channel.dart and replace _parsePossibleChannels(raw) → parsePossibleChannels(raw); delete the private method body.

Missing explicit import for UspResultParser and sealed subtypes — 🟢High [both reviewers]

lib/page/wifi_settings/services/usp_wifi_advanced_service.dart:1–5

UspResultParser, UspSuccess, UspPartialSuccess, UspFailure are used at lines 69–84 but the import block has no explicit usp_operation_result.dart entry. These symbols resolve today via transitive re-export from usp_client.dart. Other services (usp_admin_service.dart, etc.) carry explicit imports. A future re-export change breaks this file silently.

Fix: Add import 'package:privacy_gui/core/usp/models/usp_operation_result.dart';

W-NEW-1 carry — _setFailure stub key 'bulk_operation' mimics internal parser sentinel — 🟡Med [both reviewers]

test/.../usp_wifi_advanced_service_test.dart:31 — Default 'bulk_operation' matches the synthetic sentinel UspResultParser injects when success=false && error==null. Functionally correct but confusing for future maintainers. Fix: use a realistic TR-181 path (e.g. 'Device.WiFi.Radio.1.IEEE80211hEnabled').

S-1 carry — tr181_path.dart has zero unit tests — 🟢High [both reviewers]

lib/core/utils/tr181_path.dart — ensureTrailingDot is called at 14+ call sites. Three code paths (empty, already-dotted, non-dotted) are untested. Fix: add test/core/utils/tr181_path_test.dart (~15 lines).

NEW-B3 — allowPartial=false + partial-response parsing contract undocumented — 🟡Med [single, Reviewer B]

usp_wifi_advanced_service.dart:65 — With allowPartial=false, firmware should reject the whole batch; yet partial-success parsing is in place. The firmware's actual behavior (returns partial shapes regardless of the flag, per SSH testing) should be documented inline.

✅ Confirmed Resolved This Round:

• W-6 (ensureTrailingDot dedup): All 4 private copies fully removed; shared tr181_path.dart added and migrated.
• W-5 (dfsChannels5GHz regulatory scope): US/FCC UNII-2A/2C caveat + ETSI/MIC note added.
• W-2 (allowPartial contract / contradictory): Design split documented; performSave computes and forwards forceAutoChannelPaths. Reviewer B withdraws.
• possibleChannels consumer contract: WifiRadioUIModel.possibleChannels now carries explicit doc note (RAW; consumers must call filterDfsChannels).
• Result parsing in setIeee80211hEnabled: UspResultParser.parseSetResult now called; partial/complete rejections surface as typed errors.
• if (e is ServiceError) rethrow guard: Correctly prevents double-wrapping.

What looks good this round:

• tr181_path.dart extraction: all target service files and provider migrated to shared ensureTrailingDot.
• Test stubs now return well-formed WASM v0.11.0 shapes (_setSuccess/_setPartial/_setFailure).
• WifiRadioUIModel.possibleChannels doc note clear and complete.
• dfsChannels5GHz multi-market scope note added.

W-4 carry — .first on potentially-empty list → StateError — 🟢High [both reviewers]

lib/page/wifi_settings/services/usp_wifi_advanced_service.dart:76, 82

case UspPartialSuccess(failures: final f):
  throw UspPartialFailureError(
    summary:
        'IEEE80211h update partial failure: ${f.first.errorMessage}',  // StateError if f is empty
    successPaths: const [],
    failures: f,
  );
case UspFailure(errors: final e):
  throw UspCompleteFailureError(
    summary: 'IEEE80211h update failed: ${e.first.errorMessage}',      // StateError if e is empty
    failures: e,
  );

_parseGenericResult in usp_operation_result.dart enters the UspPartialSuccess branch whenever success == true && error != null. If firmware returns {'success': true, 'result': {'data': {...}, 'error': {}}} (valid shape, empty error map), error.entries is empty → failures = [] → UspPartialSuccess(failures: []) is returned → .first throws StateError. The outer catch re-throws ServiceError only; StateError falls through to mapUspErrorToServiceError, losing all diagnostic context.

Author rationale: "TOCTOU — practically unreachable." This is an internal parser invariant not enforced by the type system; any future firmware shape change can produce the empty-list path.

Fix:

case UspPartialSuccess(failures: final f):
  throw UspPartialFailureError(
    summary: f.isNotEmpty
        ? 'IEEE80211h update partial failure: ${f.first.errorMessage}'
        : 'IEEE80211h update partial failure',
    successPaths: const [],
    failures: f,
  );
case UspFailure(errors: final e):
  throw UspCompleteFailureError(
    summary: e.isNotEmpty
        ? 'IEEE80211h update failed: ${e.first.errorMessage}'
        : 'IEEE80211h update failed',
    failures: e,
  );

W-1 carry — Missing await/expectLater on new async-throw tests — 🟢High [both reviewers]

test/page/wifi_settings/services/usp_wifi_advanced_service_test.dart:191, 204

// Both new tests — permanently false-green:
expect(
  () => svc.setIeee80211hEnabled(...),
  throwsA(isA<UspPartialFailureError>()),
);  // ← no await; Future returned by the closure is never awaited

Austin acknowledged this in the inline comments added to this PR (lines 191 and 204): "this expect(fn, throwsA(...)) call where fn returns a Future is permanently false-green." The fix was not included in this commit. Both tests pass unconditionally — they provide zero coverage guarantee for the new error paths.

Fix: await expectLater(() => svc.setIeee80211hEnabled(...), throwsA(isA<UspPartialFailureError>())); (and same for the UspCompleteFailureError test).

W-3 carry — successPaths: const [] still present — 🟢High [both reviewers]

lib/page/wifi_settings/services/usp_wifi_advanced_service.dart:77

Author's Round 2 response stated "Fixed W-3" — the fix added UspResultParser.parseSetResult result parsing, which was the original W-3 issue. However, successPaths: const [] (a separate sub-issue flagged in Round 2) remains. Impact note: _parseGenericResult currently stores synthetic 'bulk_operation' keys in the successes list rather than real radio paths, so const [] is approximately equivalent in practice. The issue is inconsistency with all other UspPartialFailureError construction sites in the codebase (which map successes → successPaths properly).

Fix: Either destructure successes: final s and map, or add an inline comment: // successPaths intentionally empty — WASM v0.11.0 parser stores synthetic keys, not real radio paths.

S-1 carry — tr181_path.dart has zero unit tests — 🟢High [both reviewers]

lib/core/utils/tr181_path.dart — ensureTrailingDot is now the canonical path normalizer called by 4 service files. Three edge cases (empty string, already dot-terminated, non-dotted) are untested. Add test/core/utils/tr181_path_test.dart (~10 lines).

NEW-B7 — _setFailure stub 'bulk_operation' key lacks protocol documentation — 🟡Med [single, Reviewer B]

test/.../usp_wifi_advanced_service_test.dart:30 — 'bulk_operation' is the WASM v0.11.0 protocol-defined key for a complete SET failure. Without a comment, future maintainers won't know it's a protocol constant. A protocol version bump would silently decouple stub from real behavior.

Fix: Add // WASM v0.11.0 protocol key for complete SET failure — update if protocol changes.

NEW-B8/NEW-B3 carry — allowPartial=false + partial-response branch undocumented — 🟡Med [single, Reviewer B]

usp_wifi_advanced_service.dart:65 — _usp.set(params) uses allowPartial=false (default). By USP spec this guarantees whole-batch rejection, making UspPartialSuccess theoretically unreachable. Yet the branch exists (correctly, per SSH testing showing firmware returns partial shapes regardless). Document this discrepancy inline so the "dead" branch is not removed.

Fix: Add a comment above the UspPartialSuccess case: // allowPartial=false per spec guarantees whole-batch rejection, but firmware may return partial shapes regardless — handle defensively.

• NEW-B4 (_parsePossibleChannels duplicate): Private method fully removed from usp_wifi_data_service.dart; all callers now use wifi_channel.dart::parsePossibleChannels. Verified with grep — 0 matches for _parsePossibleChannels in all service files.
• W-6 (ensureTrailingDot dedup): All four private _ensureTrailingDot/_withTrailingDot copies removed; tr181_path.dart::ensureTrailingDot adopted across all four services. flutter analyze clean.
• W-2 (possibleChannels contract): RAW/DFS-filtering semantics documented in WifiRadioUIModel.possibleChannels.
• W-5 (dfsChannels5GHz scope): US/FCC UNII-2A/2C scope note + multi-market warning added to wifi_channel.dart.
• NEW-W2 (forceAutoChannelPaths test coverage): Provider-level tests now cover DFS-channel force, non-DFS skip, and already-enabled skip scenarios — 164 tests pass.
• Service error guard (if (e is ServiceError) rethrow): Placement correct — typed errors thrown inside the try block are re-thrown before mapUspErrorToServiceError wraps them.